-
Notifications
You must be signed in to change notification settings - Fork 249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds timeout to pipeline start #741
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
docs/cmd/tkn_pipeline_start.md
Outdated
@@ -47,6 +47,7 @@ two parameters (foo and bar) | |||
-s, --serviceaccount string pass the serviceaccount name | |||
--showlog show logs right after starting the pipeline | |||
--task-serviceaccount strings pass the service account corresponding to the task | |||
-t, --timeout string timeout for pipelinerun (default "1h") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should set a shorthand flag for this, let's keep the shorthand flag for things that are going to be used a lot in the future...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to remove this shorthand.
Should we also remove the same from task start
to maintain consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we have shipped in a release we can't remove it :( unless we go by a deprecation process (which we still need to be documented)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold |
Close #706 This adds a timeout option to `tkn pipeline start` command timeout is string variable and value can be in format `1h10m2s` default timeout value is `1h` Signed-off-by: Pradeep Kumar <pradkuma@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
We'll start the process of removing -t
for tkn task start
this release.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chmouel, danielhelfand, piyush-garg, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@piyush-garg Can we remove the hold? Any reason for it? |
yeah that was set by piyush for my previous request to remove the /hold cancel |
Close #706
This adds a timeout option to
tkn pipeline start
commandtimeout is string variable and value can be in format
1h10m2s
default timeout value is
1h
Signed-off-by: Pradeep Kumar pradkuma@redhat.com
Changes
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make check
make generated
See the contribution guide
for more details.
Release Notes